Add MatchResult::try_absorb and split reservations (#219) by joaquinbejar · Pull Request #221 · joaquinbejar/PriceLevel

joaquinbejar · 2026-09-29T09:40:32Z

Base branch: main (was stack/1-217-poisoned-match-error; #220 is merged)
Stacked on: #220 (merged)
Blocks: #222
Stack: #217 → #219 → #218 (this is 2/3)

#220 is merged and this branch is rebased onto main, so the diff is now this PR only.

Summary

Lets a caller that sweeps several levels (orderbook-rs) fold each level's MatchResult into an aggregate without losing committed trades on failure and without the extra allocation it pays today.

Changes

Split reservations: MatchResult::try_reserve_trades(n) and try_reserve_filled_order_ids(n). try_reserve / try_with_capacity keep their shared capacity (issue Tune MatchResult buffer capacity for partial fills and sweeps #148 rationale unchanged).
MatchResult::try_absorb(&mut self, level: &mut MatchResult):
- Validates first: same taker id; the level was matched with what the aggregate has left (executed + remaining == self.remaining); the aggregate is not terminal (no error recorded, not Killed / Rejected).
- Then reserves, or takes the level's buffers in place. Per vector: an empty destination swaps with the level's buffer; a destination whose entries fit into the level's spare capacity is moved to the front of that buffer and swapped (one memmove); otherwise it grows with try_reserve.
- Then commits infallibly: remaining quantity, completion and outcome follow the finalize rules; the first error wins; a level's Killed / Rejected is adopted only when the aggregate has no trades and no filled ids.
- Failure ownership: on Err the aggregate is unchanged and the level result is untouched, so its committed trades stay with the caller. On success the level result is reset to a valid empty result.
- A level killed or rejected after the aggregate already has trades leaves the aggregate PartiallyFilled; stopping the sweep is the caller's decision (documented and tested).
Crate-internal TradeList::append_reserved; rustdoc with a failure-ownership example that keeps the unabsorbed level on error; CHANGELOG Added.

Performance

Allocations per aggregation (latency harness allocation pass, 2,000 reps, 2 trades + 1 filled id per level):

Levels	Reserve and copy	`try_absorb` (exact level buffers)	`try_absorb` (level buffers with spare capacity, as `match_order` sizes them)
1	2.00 (704 B)	0	0
3	3.00 (1,856 B)	3.00	0

Criterion (new "PriceLevel - Absorb" group; 8 trades + 8 filled ids per level; inputs built outside the timed region; median ns)

Case	Reserve and copy	`try_absorb`
Empty destination (swap)	76.2	6.5
Destination with a reservation (append)	67.6	31.4
Adopt into a non-empty destination (rotate)	134.1	119.2
Growth (full buffers)	133.8	94.8

At this size the rotate path (no allocation) is slower than growth on this machine, but still faster than copying.

Testing

src/execution/tests/absorb.rs: every validation refusal (aggregate unchanged, level intact), swap and front-move paths (buffer identity), destinations already holding ids or a reservation, first error wins, outcome adoption rules, kill after trades, plus a 512-case proptest over level sequences (completeness, trade sum, outcome, serde round-trip through validated())
make pre-push (incl. lint-panic), lib 902 passed; cargo build --release; three loom targets; --cfg loom lib build
Reviewed by hotpath-reviewer (P2 on outcome adoption fixed, P3s applied) and architect (doc example and naming fixed)

Closes #219

Copilot

Copilot review overview

🟡 Changes recommended

The reservation example is not safe for replenishing makers, and exact-capacity tests and benchmarks rely on an allocator guarantee Rust does not provide.

Review effort: Balanced
Findings: 3

Open (3)

Benchmark relies on allocator-specific exact capacity · New
Trade count can exceed order count · New
Exact capacity assertion depends on allocator behavior · New

What changed in this PR

Adds allocation-aware multi-level MatchResult aggregation and includes the stacked poisoned-level reporting changes.

Changes:

Adds split reservations and MatchResult::try_absorb.
Optimizes buffer merging through append/adopt plans.
Reports poisoned-level match refusals with comprehensive tests and documentation.

File	Description
`tests/loom/fok_handoff.rs`	Adds Loom-compatible poison query.
`src/utils/alloc.rs`	Adds vector merge planning.
`src/price_level/tests/poisoned_match.rs`	Tests poisoned matching behavior.
`src/price_level/tests/mod.rs`	Registers poison tests.
`src/price_level/mod.rs`	Adds standard-lock poison query.
`src/price_level/level.rs`	Reports poisoned match failures.
`src/price_level/fok_guard.rs`	Exposes guard poison status internally.
`src/lib.rs`	Adds migration documentation.
`src/execution/tests/mod.rs`	Registers absorb tests.
`src/execution/tests/capacity.rs`	Tests split reservations.
`src/execution/tests/absorb.rs`	Tests aggregation semantics and buffers.
`src/execution/match_result.rs`	Implements reservations and absorption.
`src/execution/list.rs`	Adds allocation-free trade-list merging.
`README.md`	Regenerates migration documentation.
`doc/panic-boundaries.md`	Documents poison handling.
`CHANGELOG.md`	Records both feature sets.
`benches/latency/alloc_measurements.rs`	Measures aggregation allocations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov-commenter · 2026-09-29T09:47:03Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/execution/list.rs	`87.50% <100.00%> (+1.34%)`	⬆️
src/execution/match_result.rs	`97.86% <100.00%> (+0.63%)`	⬆️
src/utils/alloc.rs	`85.39% <100.00%> (+3.44%)`	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

joaquinbejar

REQUEST_CHANGES. The absorption implementation preserves ownership on refusal, but its public sweep example drops earlier committed trades on an error. The new buffer adoption path also needs the Criterion timing evidence required by the repository rules. Formatting, the production panic gate, Clippy, workspace tests, release absorption tests, release build and the allocation smoke test pass. The iceberg reservation bound is already covered by an existing inline comment, so I have not repeated it.

Add MatchResult::try_reserve_trades and try_reserve_filled_order_ids so a caller can size each vector on its own; a taker that only partially fills its makers no longer has to allocate a filled-id buffer. try_reserve and try_with_capacity keep their shared-capacity behaviour (#148). Also repairs the #217 CHANGELOG entry for is_poisoned (missing word, long line).

…219) Fold a later level's MatchResult for the same taker into an aggregate. Validation (taker id, the level matched with the aggregate's remaining quantity, aggregate not terminal) and reservation happen before anything moves, so on any Err the aggregate is unchanged and the level result still holds its committed trades. Per vector, an empty aggregate buffer that is too small is swapped with the level's (no allocation); otherwise entries are appended into the reserved buffer (TradeList::append_reserved). Outcome and completion are recomputed with the finalize rules, a level's Killed / Rejected is adopted only when nothing traded, and the level's error becomes the aggregate's. The drained level is left as a consistent empty result. Allocation pass (PL_LATENCY_ONLY=alloc cargo bench --bench latency), two trades + one filled id per level: 1 level: copy 2.00 allocs/op (704 B), absorb 0.00 3 levels: copy 3.00 allocs/op (1,856 B), absorb 3.00 (1,856 B)

- Adopt a level's Killed / Rejected only when the aggregate then holds no trades AND no filled ids; otherwise NotFilled. - Per vector merge plan (utils::alloc::plan_merge / merge_planned): append into the aggregate's spare capacity, else adopt the level's buffer when it holds both sets of entries (aggregate entries rotated to the front in place, no allocation), else grow. All fallible work stays before the infallible commit. - Refusal messages are built inside #[cold] constructors. - Doc example keeps an unabsorbed level result on Err and uses ?; document the caller reservation pattern and that a killed / rejected level after earlier trades leaves the aggregate PartiallyFilled and open. - Tests renamed to test_<unit>_<scenario>_<expected>; new tests for the adopt path with a non-empty aggregate, a kill into ids-without-trades and a kill after trades; proptest covers caller-added ids, reservations and level capacities. Allocation pass, allocs/op (2 trades + 1 filled id per level): 1 level: copy 2.00, absorb 0, absorb with spare level buffers 0 3 levels: copy 3.00, absorb 3.00 (exact level buffers), absorb 0 (spare level buffers), absorb+reserve 3.00

try_with_capacity only guarantees a lower bound, so the growth and adopt tests and the EXACT allocation scenario now fill buffers to their observed capacity instead of asserting an exact size.

…ult on error Reserve for the remaining quantity (order_count is advisory only: iceberg and reserve makers replenish and trade again) and return an error struct carrying the aggregate and the unabsorbed level result.

…-copy Covers empty destination (swap), existing reservation (append), adoption into a non-empty destination (rotate) and growth, inputs built outside the timed region.

joaquinbejar · 2026-09-29T10:11:54Z

Thanks. The example keeps both the aggregate and the unabsorbed result on error, the fixtures no longer rely on exact allocator sizing, and the Criterion numbers for every absorb path are in the description.

…rb bench

joaquinbejar added the enhancement New feature or request label Sep 29, 2026

joaquinbejar self-assigned this Sep 29, 2026

joaquinbejar mentioned this pull request Sep 29, 2026

Report a poisoned level through the MatchResult error slot (#217) #220

Merged

3 tasks done

joaquinbejar requested a balanced review from Copilot September 29, 2026 09:40

joaquinbejar mentioned this pull request Sep 29, 2026

Expose match-refusing counter headroom so callers can pre-flight multi-level FOK #218

Closed

9 tasks

Copilot started reviewing on behalf of joaquinbejar September 29, 2026 09:41

View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread benches/latency/alloc_measurements.rs Resolved

Comment thread src/execution/match_result.rs Resolved

Comment thread src/execution/tests/absorb.rs Resolved

joaquinbejar commented Sep 29, 2026

View reviewed changes

Comment thread src/execution/match_result.rs Resolved

Comment thread src/utils/alloc.rs Resolved

joaquinbejar mentioned this pull request Sep 29, 2026

Expose match requirements and counter headroom for multi-level FOK pre-flight (#218) #222

Merged

3 tasks done

joaquinbejar added 6 commits September 29, 2026 12:04

address review: build genuinely full buffers for absorb growth fixtures

81b3e52

try_with_capacity only guarantees a lower bound, so the growth and adopt tests and the EXACT allocation scenario now fill buffers to their observed capacity instead of asserting an exact size.

address review: Criterion group comparing try_absorb with reserve-and…

6b600a0

…-copy Covers empty destination (swap), existing reservation (append), adoption into a non-empty destination (rotate) and growth, inputs built outside the timed region.

joaquinbejar force-pushed the stack/2-219-match-result-absorb branch from b2f035e to 6b600a0 Compare September 29, 2026 10:11

fix(bench): use panic! instead of the denied unreachable! in the abso…

5877ec7

…rb bench

joaquinbejar merged commit 52cdcad into main Sep 29, 2026
13 checks passed

joaquinbejar deleted the stack/2-219-match-result-absorb branch September 29, 2026 10:23

joaquinbejar mentioned this pull request Sep 29, 2026

Release 0.10.1 #223

Merged

2 tasks done

Navigation Menu

Repository navigation

Add MatchResult::try_absorb and split reservations (#219) - #221

Add MatchResult::try_absorb and split reservations (#219)#221
joaquinbejar merged 7 commits into
mainfrom
stack/2-219-match-result-absorb

joaquinbejar commented Sep 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 29, 2026 •

edited

Loading

Uh oh!

joaquinbejar left a comment

Uh oh!

Uh oh!

Uh oh!

joaquinbejar commented Sep 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

joaquinbejar commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Performance

Criterion (new "PriceLevel - Absorb" group; 8 trades + 8 filled ids per level; inputs built outside the timed region; median ns)

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joaquinbejar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joaquinbejar commented Sep 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joaquinbejar commented Sep 29, 2026 •

edited

Loading

codecov-commenter commented Sep 29, 2026 •

edited

Loading